Fix incomplete submit #11
Conversation
|
Hey Marcel, thank you for submitting the new PR. Can I have the new deploy preview for this PR? |
frontend/modules/Home/index.tsx
Outdated
| else if (url === "" && alias === "") { | ||
| toast({ | ||
| title: "Error occured", | ||
| description: "Please enter long url and short url", | ||
| status: "error", | ||
| duration: 5000, | ||
| isClosable: true, | ||
| }); | ||
| } else if (alias === "") { | ||
| toast({ | ||
| title: "Error occured", | ||
| description: "Please enter short url", | ||
| status: "error", | ||
| duration: 5000, | ||
| isClosable: true, | ||
| }); | ||
| } else if (url === "") { |
There was a problem hiding this comment.
can we refactor this? the if statements are quite confusing and it's not readable. One thing to consider is to use a variable for each condition for example:
if (alias === ''){}to
const isAliasEmpty = alias === ''and then you can refactor the ifs into a more elegant way like:
if (isAliasEmpty){
toast({title: "Alias Empty"})
}
frontend/modules/Home/index.tsx
Outdated
|
|
||
| useEffect(() => { | ||
| if (!!alias && isUrlValid) { | ||
| if (!!alias && isUrlValid && url !== "") { |
There was a problem hiding this comment.
Refactor these boolean conditions into a more readable variable e.g. urlValid
frontend/package.json
Outdated
| @@ -15,6 +15,7 @@ | |||
| "axios": "^0.21.1", | |||
| "dotenv": "^9.0.0", | |||
| "framer-motion": "^4", | |||
| "gzip-cli": "^1.2.0", | |||
There was a problem hiding this comment.
why would we need gzip in our dependency? If this is unused, please remove.
|
Hello! I've redeployed this preview on Ristek Preview. I've submitted the revision too, please kindly check my commit. @jonathanfilbert |
frontend/modules/Home/index.tsx
Outdated
| toast({ | ||
| title: "Error occured", | ||
| title: "Long url and short url empty", | ||
| description: "Please enter long url and short url", | ||
| status: "error", | ||
| duration: 5000, | ||
| isClosable: true, | ||
| }); |
There was a problem hiding this comment.
This toast function is a bit redundant. It's the same toast function but used multiple of times in this file. Is there any way we can make this simpler? Maybe wrap the toast function in a reusable component called Toast with props like:
ToastProps = {
title: String,
description: String,
isError?: Boolean, // this one defaults to false
}the duration and isClosable don't need to be exported into props because they're default behavior.
So everytime we want to call the Toast, all we have to do is just:
<Toast isError title="Long URL empty" description="Please enter long url" />
Revision of #10
bug:
Users can submit without a long URL is filled.
request feature:
Add toast to handle disabled button is clicked
old behavior :
Submit button isn't disabled and users can submit without a long URL, when users trying to click the disabled button there's no toast pop up
new behavior :
You can try to submit without long URL and short URL, short URL only, long URL only on RISTEK-LINK-PREVIEW
Thanks 😀